Skip to content

fix(github): detect rate limits using response headers#1

Closed
Ridanshi wants to merge 117 commits into
mainfrom
fix/daily-focus-server-persistence
Closed

fix(github): detect rate limits using response headers#1
Ridanshi wants to merge 117 commits into
mainfrom
fix/daily-focus-server-persistence

Conversation

@Ridanshi
Copy link
Copy Markdown
Owner

@Ridanshi Ridanshi commented Jun 2, 2026

Closes Priyanshu-byte-coder#1741

Root cause

The project uses GitHub OAuth App (not GitHub App), so tokens have no refresh mechanism — they remain valid until a user revokes access in GitHub Settings. A 30-day JWT session meant a revoked token could silently serve empty dashboards for up to 30 days.

The auth layer already had a 24-hour periodic liveness check that marks session.error = "TokenRevoked" when GitHub returns 401, and lib/github-fetch.ts already exported GitHubAuthError and githubAuthErrorResponse(). However, 13 metric routes were missing one or both of:

  1. The session.error === "TokenRevoked" early-exit guard
  2. Throwing GitHubAuthError on 401 responses inside inner fetch calls
  3. Catching GitHubAuthError and returning githubAuthErrorResponse() instead of a generic 502

Authentication model discovered

  • Provider: GitHub OAuth App via NextAuth (no refresh tokens)
  • Token storage: JWT cookie (30-day maxAge), access token embedded in the JWT
  • Revocation detection: periodic 24-hour liveness check in the JWT callback; also live detection when any API route returns 401
  • Session flag: session.error = "TokenRevoked" surfaced via NextAuth session callback; checked at route entry to short-circuit before any GitHub API call

API behaviour changes

All affected routes now return a structured error instead of silently degrading:

{ "error": "token_expired" }

with HTTP 401.

Routes updated: achievements, coding-activity-insights, contributions/daily, contributions/hourly, repo-explorer, weekly-summary, discussions, ci, pinned-repos, pr-breakdown, pr-review-time, inactive-repos, productive-hours

Non-auth failures (rate limit 403, 422, 5xx) still return { "error": "GitHub API error" } with HTTP 502 — unchanged.

Dashboard changes

  • InactiveRepositoriesCard and ProductiveHoursWidget: detect token_expired in the API response and render an inline reconnect prompt
  • GitHubTokenExpiredBanner (already present): shown when session.error === "TokenRevoked" or when the dashboard fetch interceptor fires github-token-expired
  • The banner message: "Your GitHub connection is no longer valid. Please reconnect your GitHub account to continue syncing data."

Tests added

  • test/github-auth-metrics.test.ts — 15 tests for discussions, pr-review-time, productive-hours, inactive-repos, and weekly-summary covering: TokenRevoked session path, GitHub 401 propagation, and non-auth failures staying as 502
  • test/token-expired.test.ts — extended with coverage for discussions, pinned-repos, pr-breakdown, and weekly-summary routes

Verification commands

npm run lint      # passes (pre-existing warnings only, unrelated to this change)
npm run type-check # passes with no errors  
npm test          # 87 test files, 1285 tests, all passing

YuktiNandwana and others added 30 commits June 1, 2026 00:22
Priyanshu-byte-coder#357)

* feat: add review cycle time metric with weekly trend chart and slowest repos

* fix: resolve all review regressions, fix PR count, restore combined-accounts and query optimization
* Add repository contribution distribution chart

* Add repository contribution distribution chart

* Fix repository chart theme styles and env tracking

* Address repository chart review fixes

* Replace raw error colors with theme variables

* style(chart): resolve recharts colors dynamically from theme-aware CSS variables
…nc (Priyanshu-byte-coder#1783)

Root cause
----------
The goals sync route read an optional repository filter from stored goal
rows and interpolated the raw value directly into a GitHub Search API
URL:

  const repo =
    (goal as any).repo ||
    (goal as any).repository ||
    (goal as any).repo_name ||
    null;

  const repoQualifier = repo ? `+repo:${repo}` : "";
  fetch(`…/search/commits?q=author:${login}${repoQualifier}+author-date:…`)

A stored value such as "octocat/Hello-World+author:victim" would inject
an additional author qualifier into the GitHub Search API query, silently
expanding the commit search beyond the authenticated user's own history
and producing incorrect goal progress counts.

Additionally, `(goal as any)` bypassed TypeScript type safety across all
three field name variants, and the raw query string used `+` concatenation
instead of URLSearchParams, making the parameter boundary unclear.

Fix
---

src/app/api/goals/sync/route.ts

- Introduce REPO_IDENTIFIER_RE — a strict regex accepting only
  "owner/repo" where owner is 1–39 chars (alphanumeric + hyphens,
  no leading/trailing hyphens) and repo is 1–100 chars (alphanumeric,
  dots, hyphens, underscores). The same constraint previously applied
  to the public repo-analytics endpoint.

- Export extractValidRepoFromGoal(goal: ActivityGoal): string | null
  which reads the first non-null value from goal.repo → goal.repository
  → goal.repo_name, trims it, validates it against the regex, and
  additionally rejects "." and ".." as repo names. Any value that does
  not match returns null so the repo filter is omitted from the query.

- Define the ActivityGoal typed interface to replace (goal as any),
  documenting the three optional field names and providing proper type
  coverage for downstream code.

- Move GitHub Search URL construction to URLSearchParams so the combined
  qualifier string is encoded as a single atomic value and cannot be
  split by embedded special characters. Applied to both the commit
  search loop and the PR search call.

test/goals-sync-repo-validation.test.ts — 28 new tests:
  Valid inputs: standard, dots/underscores, org hyphens, all three
    field names, whitespace trimming, max lengths
  Null/empty: all-null, empty string, whitespace-only
  Query injection (regression for Priyanshu-byte-coder#1757): embedded +author:, space-
    separated qualifier, ampersand, URL-encoded %2B
  Extra path segments: three-segment, four-segment paths
  Path traversal: owner/.., owner/.
  Invalid formats: bare name, leading/trailing slash, hyphen rules,
    owner >39 chars, repo >100 chars
  Field priority: repo > repository > repo_name; invalid higher-
    priority field does not fall through to a valid lower-priority field

Closes Priyanshu-byte-coder#1757
…shu-byte-coder#1544)

* test: fix broken unit test suite and failing assertions

* fix(auth): resolve unauthorized redirect loops and fix e2e test failures

* chore: remove debug files fix.js and fix2.js

* fix: resolve duplicated code in GoalTracker.tsx
… Improvements (Priyanshu-byte-coder#1549)

* test: fix broken unit test suite and failing assertions

* feat(a11y): implement comprehensive accessibility improvements

* chore: remove patch_project_metrics.py and relax jsx-a11y rules

* fix: resolve duplicated code in GoalTracker.tsx

* fix: resolve accessibility label error
…hu-byte-coder#1655)

* feat(Priyanshu-byte-coder#964): add theme toggle to AppNavbar for public profile and all non-dashboard pages

- Add ThemeToggle to AppNavbar desktop nav and mobile menu
- Conditionally hidden on /dashboard/* routes where DashboardHeader already provides it
- Ensures unauthenticated visitors on /u/:username can toggle dark/light mode
- Theme preference persisted to localStorage via existing ThemeContext

E2E test coverage:
- Add 'public profile page theme toggle works without authentication' test in theme.spec.js
- Scope contribution graph range button click to #contribution-activity (strict mode fix)
- Fix notifications spec: use correct NEXTAUTH_SECRET and proper mock shapes
- Fix theme spec: use .first() selector to handle multiple ThemeToggle instances

Closes Priyanshu-byte-coder#964

* fix(theme): ensure light mode is consistently applied across all pages

- Replaced hardcoded #050505 background with var(--background) in globals.css for the landing page
- Swapped hardcoded dark theme colors (borders, text, backgrounds) in LandingPage.tsx to use CSS variables from globals.css
- Updated AppNavbar scrolled state and mobile menu background to use color-mix with var(--background) instead of hardcoded rgba dark colors

This ensures that when a user switches to light mode, the landing page and navbar fully respect the theme.

* fix(theme): make footer respect light mode

- Removed forced 'dark' class from the global Footer
- Replaced hardcoded #e8e8e8 and white text with var(--foreground)
- Replaced hardcoded #9ca3af with var(--muted-foreground)

This fixes the issue where the footer was stuck in dark mode on some pages or had invisible white-on-white text on the landing page in light mode.

* refactor: remove stale LandingNav and LandingFooter

* fix(test): update landing footer e2e test selector
…byte-coder#1658)

* feat: add dynamic scroll progress ring to Back to Top button

* feat: smooth scroll animation for navbar anchor links
Priyanshu-byte-coder#1660)

* feat: implement resilient React Error Boundaries for critical dashboard widgets

* feat: wrap dashboard widgets with WidgetErrorBoundary in page.tsx
…riyanshu-byte-coder#1684)

* refactor: centralize streak calculation (Priyanshu-byte-coder#1434)

* fix: import toDateStr in weekly summary route

* fix: disable secure NextAuth cookies in Playwright server mode

* fix: restore default NextAuth cookie handling for Playwright E2E

* fix: force non-secure NextAuth cookies in Playwright server mode

* fix: allow Playwright E2E session cookie through middleware

* fix: repair JSX/TS syntax blocking CI

* fix: resolve dashboard page syntax errors

* fix: resolve merge conflicts with upstream/main in streak-unification PR

* fix(e2e): resolve strict mode violation for Goals heading locator

getByRole('heading', { name: 'Goals' }) matched two elements:
1. <h2>Goals & Insights</h2> (section heading from dashboard restructure)
2. <h2>Goals</h2> (GoalTracker heading)

Adding exact: true matches only the GoalTracker heading and avoids
the Playwright strict mode violation.
…er#1705)

* chore: clean up and refresh leaderboard branch

* fix: restore missing leaderboard data file

* fix: replace file with clean, conflict-free code

* fix: resolve manual merge conflicts and remove markers
…oder#1707)

* fix: replace tiny dropdown chevron with proper icon

* fix: resolve merge conflicts and update dropdown chevron
* fix: make wrapped loading skeleton responsive

* fix: prevent long repository names from breaking wrapped slide layout
…nshu-byte-coder#1718)

* fix(security): remove internal data from webhook POST response

Strip verbose response payload from the GitHub webhook handler that
exposed internal user IDs, account types, and system metadata to
external callers via GitHub's webhook delivery logs.

All three response paths now return a minimal { received: true }
acknowledgment. Internal processing details remain logged server-side
via the existing logError infrastructure.

Closes Priyanshu-byte-coder#1665

* test(webhook): add response shape assertions to prevent data leakage regression

Adds three test cases verifying that the webhook POST handler does not
expose internal fields (githubId, accountType, userMatched, etc.) in
any of its response paths.
Priyanshu-byte-coder and others added 4 commits June 2, 2026 15:46
File had no 'packages' field causing ERR_PNPM_INVALID_WORKSPACE_CONFIGURATION.
This is not a monorepo; workspace config is unnecessary.
…rters

These were direct dependencies that got removed from package.json but remained
in the lockfile specifiers, causing ERR_PNPM_OUTDATED_LOCKFILE on Vercel.
…s flex row

A merged PR placed CodingActivityInsightsCard, PRReviewTrendChart, IssueMetrics,
CIAnalytics, DiscussionsWidget, PinnedReposWidget, InactiveRepositoriesCard,
TopRepos, LanguageBreakdown, GoalTracker, and RecentActivity inside the flex-row
quick-actions container, squeezing all dashboard content into one horizontal row.
All these widgets already exist in the proper sections below.
Previously, any 403 or 429 response from GitHub was classified as a
rate-limit error, causing misclassification of permission failures,
invalid credentials, and insufficient-scope errors.

Root cause: GitHubRateLimitError was thrown on HTTP status alone
without inspecting X-RateLimit-Remaining, Retry-After, or response
body content.

Changes:
- Add extractRateLimitInfo() to parse X-RateLimit-Reset,
  X-RateLimit-Remaining, and Retry-After headers
- Add isSecondaryRateLimitBody() to detect secondary rate-limit signals
  in response body messages
- Add buildGitHubError() that classifies errors correctly:
    429                           => GitHubRateLimitError
    403 + X-RateLimit-Remaining=0 => GitHubRateLimitError (primary)
    403 + Retry-After present     => GitHubRateLimitError (secondary)
    403 + rate-limit body message => GitHubRateLimitError (secondary)
    403 (all other cases)         => GitHubApiError
- Enrich GitHubRateLimitError with retryAfter field (seconds)
- Add 25 new tests covering all classification paths for both
  githubFetch and githubGraphQL
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

GSSoC Label Checklist 🏷️

@Ridanshi — please apply the appropriate labels before merging:

Difficulty (pick one):

  • level:beginner — 20 pts
  • level:intermediate — 35 pts
  • level:advanced — 55 pts
  • level:critical — 80 pts

Quality (optional):

  • quality:clean — ×1.2 multiplier
  • quality:exceptional — ×1.5 multiplier

Validation (required to score):

  • gssoc:approved — counts for points
  • gssoc:invalid / gssoc:spam / gssoc:ai-slop — does not score

Type labels (type:*) are auto-detected from files and title. Review and adjust if needed.
Points formula: (difficulty × quality_multiplier) + type_bonus

Copy link
Copy Markdown

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your first PR on DevTrack! 🎉

A maintainer will review it within 48 hours. While you wait:

  • Make sure CI is passing (type-check + lint)
  • Double-check the PR description is filled out and the issue is linked
  • Feel free to ask questions in Discussions if you need help

If you find DevTrack useful, a ⭐ star on the repo is always appreciated — it helps the project grow and attract more contributors!

Ridanshi added 15 commits June 2, 2026 20:56
Recurring goals silently erased period data when the weekly/monthly
boundary was crossed. Before this fix, current was zeroed and
period_start advanced with no record of what was achieved.

Root cause: the reset branch in GET /api/goals called
supabase.update({ current: 0, period_start }) with no prior archival.

Changes:
- Upsert into goal_history (goal_id, period_start unique) before every
  reset, capturing achieved_value, target, completed, and period dates.
  ignoreDuplicates=true makes concurrent resets idempotent.
- Fetch the most recent history row per recurring goal and attach as
  last_period in the GET response so the UI needs no extra request.
- GoalTracker shows check N/M or cross N/M last period for recurring goals.
- data-export includes full goal_history records and fixes goals column
  list; goal_history is also purged on account deletion.
- Migration creates goal_history table with referential integrity,
  cascade-on-delete, and indexes for efficient last-period lookup.
- 17 tests cover completed/incomplete resets, idempotency, ordering,
  last_period attachment, weekly, monthly, and multi-goal scenarios.
sentCount was incremented unconditionally after each loop iteration,
regardless of whether the Resend API call succeeded, failed with a
non-2xx response, or threw a network exception. The fetch() return
value was completely discarded, making the reported count misleading
and hiding delivery failures from operators.

Changes to src/app/api/cron/weekly-digest/route.ts:
- Capture the Resend fetch() response in a local variable
- Check res.ok before incrementing sentCount
- Add failedCount to track sends that received non-2xx responses
- Wrap each per-user fetch in try/catch so a network error for one
  recipient does not abort delivery to subsequent recipients
- Log failed sends with HTTP status and github_login (not email)
- Include failedCount in the JSON response alongside sentCount

Changes to test/weekly-digest-cron-auth.test.ts:
- Update existing success test to assert failedCount: 0
- Update RESEND_API_KEY-absent test: sentCount is now 0 (not 1)
  since no delivery was actually attempted
- Add 8 new tests covering: successful delivery, Resend 422/429/500
  error responses, network exceptions, mixed success/failure batches,
  per-user failure isolation, and response shape
Closes Priyanshu-byte-coder#1878

## Vulnerability analysis

NextAuth session cookies are SameSite=Lax by default, which blocks
naive cross-site form submissions but does not prevent attacks from
same-registered-domain subdomains or certain browser edge cases.
State-changing API routes authenticated only via cookies were
therefore vulnerable to CSRF.

## Root cause

No Origin/Referer validation existed for browser-authenticated
mutations. API routes accepted POST/PUT/PATCH/DELETE requests from
any origin as long as a valid session cookie was present.

## Implementation

**Layer 1 — Origin validation (src/lib/security/csrf.ts)**
- `buildAllowedOrigins()` reads NEXTAUTH_URL and ALLOWED_ORIGINS env
  vars to build the permitted-origin set.
- `getRequestOrigin()` extracts the Origin header, falling back to the
  Referer origin when Origin is absent.
- `isOriginAllowed()` performs exact, case-sensitive comparison.
- `isCsrfExemptPath()` marks paths that authenticate out-of-band.

**Layer 2 — Middleware enforcement (src/middleware.ts)**
- Applies to all authenticated (token present) non-safe-method requests
  under /api/ that are not on the exempt list.
- Requests carrying Authorization: Bearer are skipped — they are API
  clients, not browser sessions, and are not susceptible to CSRF.
- Cross-origin requests are rejected with 403.

**Exempt paths (authenticate via their own mechanism)**
- /api/webhooks/github — x-hub-signature-256 HMAC
- /api/webhooks/dispatch — WEBHOOK_DISPATCH_SECRET bearer
- /api/cron/ — CRON_SECRET bearer
- /api/auth/ — NextAuth internals

**Allowed-origin configuration (.env.example)**
- NEXTAUTH_URL is always included automatically.
- ALLOWED_ORIGINS accepts additional comma-separated origins for
  staging/preview deployments.

## Protected routes (state-changing, session-authenticated)

POST:   /api/goals, /api/rooms, /api/notifications,
        /api/webhooks/custom, /api/daily-focus (via PUT)
PUT:    /api/daily-focus
PATCH:  /api/goals/[id], /api/user/settings,
        /api/notifications/[id]
DELETE: /api/goals/[id], /api/streak/freeze,
        /api/daily-focus, /api/notifications/[id]

## Additional fixes

- goals/[id] PATCH: require current as a non-negative integer
  (reject missing field and floats)
- local-coding/stats: return 500 on DB error instead of empty stats
- user/github-accounts: return 500 on DB error instead of empty list
- discord.test.ts: update test URLs to valid Discord webhook format
  so they pass the URL validation added to discord.ts

## Tests added

test/csrf.test.ts — 42 tests covering:
  - SAFE_METHODS set membership
  - isCsrfExemptPath for all exempt and non-exempt paths
  - buildAllowedOrigins with NEXTAUTH_URL, ALLOWED_ORIGINS, both, neither
  - getRequestOrigin: Origin header, Referer fallback, both present,
    malformed Referer, whitespace trimming
  - isOriginAllowed: known/unknown/scheme/port/case/empty
  - Middleware integration: valid origin, invalid origin, missing
    headers, unauthenticated bypass, safe methods, bearer bypass,
    webhook exemption, cron exemption, Referer fallback

test/daily-focus.test.ts — 19 tests for the new /api/daily-focus route

## Verification

pnpm test   — 1169/1169 passed
pnpm lint   — no errors (warnings pre-existing)
pnpm typecheck — clean
Closes Priyanshu-byte-coder#1878

Include remaining test and component files for the security branch.
Closes Priyanshu-byte-coder#1878

Update github auth error response code for consistency with client handling.
Closes Priyanshu-byte-coder#1878

Update component error handling and auth error test assertions.
Closes Priyanshu-byte-coder#1878

Commit the remaining SSE stream connection improvements and date-utils
re-export that were staged alongside the CSRF implementation.

- src/app/api/stream/route.ts: add heartbeat keepalive, idle-timeout
  recycling, and max-age ceiling so zombie connections do not hold
  slots indefinitely; extract HEARTBEAT_INTERVAL_MS, IDLE_TIMEOUT_MS,
  MAX_AGE_MS as named exports for test coverage
- src/lib/date-utils.ts: re-export date helpers from dateUtils.ts under
  the hyphenated alias expected by test/date-utils.test.ts
Extends the existing test file from 10 basic cases to 47 behavioral
tests, grouped into three describe blocks.

Functions covered:
- privateCacheHeaders(maxAgeSeconds?, swrSeconds?)
- publicCacheHeaders(maxAgeSeconds?, swrSeconds?)

New coverage added:

SWR default formula regression
- Verifies stale-while-revalidate defaults to exactly 2× maxAgeSeconds
  for both functions, using non-round inputs (100, 150, 3600) that
  would expose an accidental multiplier change

Directive presence / absence guards
- privateCacheHeaders: always contains `private`, never `public` or
  `s-maxage`
- publicCacheHeaders: always contains `public` and `s-maxage`, never
  `private` or standalone `max-age`

Exact format and ordering
- Regex asserts on full directive pattern
  (e.g. /^private, max-age=\d+, stale-while-revalidate=\d+$/)
- Verifies `=` assignment, comma-space separators, and the full
  `stale-while-revalidate` keyword (not an abbreviation)
- Ordering test: splits on `", "` and checks position of each part

Return object shape
- Both functions return an object with exactly one key: `Cache-Control`
- Value is a string

Large / boundary values
- 86400 s (24 h) and explicit swrSeconds=0 override

Headers constructor compatibility
- Confirms the returned HeadersInit can be passed to `new Headers()`
  and yields the correct `Cache-Control` value via `.get()`

Cross-function regression describe block
- Two functions produce distinct values for the same input
- max-age vs s-maxage mutual exclusion
- private vs public mutual exclusion
- Both expose stale-while-revalidate
- Directive count is exactly 3 for both functions
Closes Priyanshu-byte-coder#1879

Root cause: src/lib/date-utils.ts (barrel re-export) was committed
without corresponding regression tests, so a future rename of
dateDiffDays or dateDiff in the barrel could silently break callers
without any test catching it.

Fix: extend test/dateDiffDays.test.ts with a dedicated describe block
that imports both dateDiffDays and dateDiff from the @/lib/date-utils
barrel and asserts:
- both names are exported as functions
- dateDiff and dateDiffDays return identical results
- correct positive, zero, and negative day differences

Tests added:
- "exports dateDiffDays" — verifies named export exists
- "exports dateDiff alias" — verifies alias export exists
- "dateDiff and dateDiffDays produce identical results"
- "barrel dateDiffDays computes correct day difference"
- "barrel dateDiff computes correct day difference"
- "barrel export returns 0 for same date"
- "barrel export returns negative for reversed dates"

Verification:
  next lint   → warnings only (pre-existing), no errors
  tsc --noEmit → clean
  vitest run  → 1255 tests across 86 files, all passed
Closes Priyanshu-byte-coder#1848

Root cause: Today's Focus goal was stored only in localStorage, making
it inaccessible across devices, browsers, and incognito sessions. Any
browser storage clear or device switch would lose the user's focus entry.

Implementation summary:

Database layer
- Added migration 20260602000000_add_daily_focus.sql creating the
  daily_focus table with columns (id, user_id, focus_date, goal_text,
  created_at, updated_at), a unique (user_id, focus_date) constraint,
  composite index, FK to users with ON DELETE CASCADE, and RLS policies
  scoped to auth.uid()::text so each user can only access their own rows.

API layer
- Implemented GET, PUT (upsert), and DELETE handlers under
  src/app/api/daily-focus/route.ts following the same authentication,
  validation, and error-handling patterns used throughout the codebase.
  Input is validated and sanitised via validateTextInput(); date params
  are validated with a strict regex before use.

Frontend
- Refactored src/components/TodayFocusHero.tsx to treat the server as
  the source of truth. On mount the component fetches today's record;
  saves and deletes are sent to the API with optimistic updates and
  rollback on failure. If the server is unreachable the component falls
  back to localStorage and shows a sync warning. Unauthenticated users
  retain the localStorage-only experience unchanged.

Migration path
- On first load, if no server record exists but a localStorage entry
  does, the component silently migrates the value via PUT and removes
  the localStorage key only after a confirmed 200 response. No data is
  lost during the transition.

Export support
- src/app/api/user/data-export/route.ts now includes a dailyFocus
  section (last 365 records) in GET exports and explicitly deletes
  daily_focus rows during account deletion.

Additional fixes included in this branch
- Extended GitHub auth-error propagation to ci, inactive-repos,
  pinned-repos, productive-hours, and weekly-summary metric routes, and
  to the ci-analytics helper, using the shared GitHubAuthError /
  githubAuthErrorResponse pattern already established earlier on this
  branch.
- Updated DiscussionsWidget and PRReviewTrendChart to handle
  token_expired responses from the API.

Tests added
- test/daily-focus.test.ts: 19 tests covering GET (authenticated,
  unauthenticated, missing record, existing record, DB error), PUT
  (create, upsert, validation errors, invalid JSON, DB error), and
  DELETE (success, idempotent delete, defaults, DB error).

Verification commands
  next lint     -- warnings only (pre-existing), no errors
  tsc --noEmit  -- clean
  vitest run    -- all tests pass
…ric routes

Extends the GitHubAuthError / githubAuthErrorResponse pattern to
coding-activity-insights, pr-breakdown, PRReviewTrendChart, and
WeeklySummaryCard — completing the token-revocation error propagation
pass started in earlier commits on this branch.
The SSE stream endpoint previously polled two database tables every 2 s
per open connection with no cap on concurrent connections and no cleanup
of zombie connections, creating O(N*queries) database load.

The fix (already on branch in c3f4c6b) addressed this with:
- Per-user connection cap of 4 (HTTP 429 with Retry-After when exceeded)
- Poll interval increased from 2 s to 15 s (7.5x fewer queries)
- Events only emitted when data actually changes (no client spam)
- Heartbeat keepalive every 30 s to detect dead TCP streams early
- Idle timeout: connections with no real data for 5 min are recycled
- Max-age ceiling: connections forcibly recycled after 30 min
- Single closeStream() path with a cleanedUp guard prevents slot
  double-decrement when abort, idle-timeout, and max-age race

This commit adds dedicated test coverage for the lifetime-control
mechanisms that were previously untested:
- HEARTBEAT_INTERVAL_MS / IDLE_TIMEOUT_MS / MAX_AGE_MS constant values
- Idle timeout releases the connection slot (fake-timer test)
- Max-age releases the connection slot (fake-timer test)

All 20 SSE stream tests pass.
Complete the remaining auth-error propagation gaps so all metrics
routes and dashboard widgets behave consistently when a GitHub OAuth
token is revoked.

Routes: add TokenRevoked early-exit + GitHubAuthError catch to
achievements, coding-activity-insights, contributions/daily,
contributions/hourly, and repo-explorer. InactiveRepositoriesCard
and ProductiveHoursWidget detect token_expired and render a
reconnect prompt instead of a generic error state.

Tests: new github-auth-metrics.test.ts covers all five newly-fixed
routes for the TokenRevoked session path, GitHub 401 propagation,
and non-auth failures remaining as 502. token-expired.test.ts
extended with matching coverage for discussions, pinned-repos,
pr-breakdown, and weekly-summary routes.
@Ridanshi Ridanshi force-pushed the fix/daily-focus-server-persistence branch from 8377f98 to d973cda Compare June 2, 2026 16:00
@Ridanshi
Copy link
Copy Markdown
Owner Author

Ridanshi commented Jun 4, 2026

This PR was targeting the fork's internal main branch instead of upstream. The work for issue Priyanshu-byte-coder#1741 has been rebased onto the latest upstream main and resubmitted as Priyanshu-byte-coder#2040. This fork PR can be closed once the upstream PR is reviewed and merged.

@Ridanshi
Copy link
Copy Markdown
Owner Author

Ridanshi commented Jun 4, 2026

Closing in favour of the properly targeted upstream PR: Priyanshu-byte-coder#2040

@Ridanshi Ridanshi closed this Jun 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet